Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service overview e2e test #125359

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

MiriamAparicio
Copy link
Contributor

Added e2e test for service overview as part of #116417

when navigating to the service overview

  • the transaction type selector is populated
  • the transaction latency chart is rendered
  • the throughput chart is rendered
  • the transaction group table is rendered
  • the failed transaction rate chart is rendered
  • the errors table is rendered
  • the breakdown chart is rendered
  • the dependencies table is rendered
  • the instances latency distribution chart is rendered
  • the instances table is rendered
  • API calls are made

and selecting a different environment

  • API calls are made with the selected environment

and clicking the refresh button

  • API calls are made with the updated time range

and selecting a different time range

  • API calls are made with the updated time range
    and clicking the refresh button
    • API calls are made with the updated time range

and selecting a different comparison window

  • API calls are made with the updated comparison time range

@MiriamAparicio MiriamAparicio added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.1.0 labels Feb 11, 2022
@MiriamAparicio MiriamAparicio requested a review from a team as a code owner February 11, 2022 11:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@MiriamAparicio
Copy link
Contributor Author

@elasticmachine merge upstream

Comment on lines 199 to 231
it('when clicking the refresh button', () => {
cy.wait(aliasNames, { requestTimeout: 10000 });
cy.contains('Refresh').click();
cy.wait(aliasNames);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it expect something after clicking on refresh?

Comment on lines 205 to 245
it('when selecting a different time range and clicking the update button', () => {
cy.wait(aliasNames, { requestTimeout: 10000 });

cy.selectAbsoluteTimeRange(
'Oct 10, 2021 @ 01:00:00.000',
'Oct 10, 2021 @ 01:30:00.000'
);
cy.contains('Update').click();
cy.wait(aliasNames, { requestTimeout: 10000 });

cy.contains('Refresh').click();
cy.wait(aliasNames, { requestTimeout: 10000 });
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well?

Maybe you could do something like:

cy.expectAPIsToHaveBeenCalledWith({
        apisIntercepted: aliasNames,
        value: 'start=xxxx&end=zzz
      });

Comment on lines 219 to 260
it('when selecting a different comparison window', () => {
cy.get('[data-test-subj="comparisonSelect"]').should('have.value', 'day');

// selects another comparison type
cy.get('[data-test-subj="comparisonSelect"]').select('week');
cy.get('[data-test-subj="comparisonSelect"]').should(
'have.value',
'week'
);
cy.wait(aliasNames, { requestTimeout: 10000 });
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here:

cy.expectAPIsToHaveBeenCalledWith({
        apisIntercepted: aliasNames,
        value: 'comparisonStart=...
      });

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do it just for the endpoint that use the comparison, I'm going to split the request in two lists

Comment on lines 183 to 184
<div data-test-subj="serviceOverviewErrorsTable">
<EuiFlexGroup direction="column" gutterSize="s">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this extra div really necessary?

Suggested change
<div data-test-subj="serviceOverviewErrorsTable">
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexGroup direction="column" gutterSize="s" data-test-subj="serviceOverviewErrorsTable">

Comment on lines 148 to 149
<div data-test-subj="serviceOverviewInstancesTable">
<EuiFlexGroup direction="column" gutterSize="s">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div data-test-subj="serviceOverviewInstancesTable">
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexGroup direction="column" gutterSize="s" data-test-subj="serviceOverviewInstancesTable">

Comment on lines 208 to 209
<div data-test-subj="dependenciesTable">
<EuiFlexGroup direction="column" gutterSize="s">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div data-test-subj="dependenciesTable">
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexGroup direction="column" gutterSize="s" data-test-subj="dependenciesTable">

Comment on lines 232 to 233
<div data-test-subj="transactionsGroupTable">
<EuiFlexGroup direction="column" gutterSize="s">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div data-test-subj="transactionsGroupTable">
<EuiFlexGroup direction="column" gutterSize="s">
<EuiFlexGroup direction="column" gutterSize="s" data-test-subj="transactionsGroupTable">

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some suggestions, let me know what you think.

Copy link
Contributor

@gbamparop gbamparop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding all these tests! I've added some comments on the PR

@MiriamAparicio
Copy link
Contributor Author

@elasticmachine merge upstream

@MiriamAparicio MiriamAparicio force-pushed the service-overview-2e2-test branch from 07676c2 to 2a63a21 Compare February 14, 2022 14:17
'have.value',
'Worker'
);
describe('transactions', () => {
Copy link
Contributor Author

@MiriamAparicio MiriamAparicio Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept these tests for the transactions because they were already there, but I think we need to create a specific test file for the transactions flow
What do you think @cauemarcondes @gbamparop ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or test all the tabs flow here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see, where are testing if the transaction type is persisted when navigating from service overview to something else, so IMO it makes sense to keep it here. What you could do is to improve the describe title.

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sorenlouv sorenlouv changed the title Service overview 2e2 test Service overview e2e test Feb 14, 2022
@MiriamAparicio
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 2.7MB 2.7MB +175.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@gbamparop gbamparop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@MiriamAparicio MiriamAparicio merged commit 0e87ffc into elastic:main Feb 15, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 15, 2022
@MiriamAparicio MiriamAparicio deleted the service-overview-2e2-test branch February 15, 2022 11:07
@kibanamachine
Copy link
Contributor

The following labels were identified as gaps in your version labels and will be added automatically:

  • v8.2.0

If any of these should not be on your pull request, please manually remove them.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 15, 2022
* Add e2e test for service overview

* add test for comparison type change

* PR review comments addressed

* improve time range test

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 0e87ffc)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

jloleysens added a commit to jloleysens/kibana that referenced this pull request Feb 15, 2022
…pdf-generation-in-worker-thread

* 'main' of github.com:elastic/kibana: (40 commits)
  Service overview e2e test (elastic#125359)
  [Graph] Make graph edges easier to click (elastic#124053)
  [Reporting] Add additional PNG and PDF metrics  (elastic#125450)
  [APM] Lint rule for explicit return types (elastic#124771)
  [SecuritySolution] Enrich threshold data from correct fields (elastic#125376)
  [Uptime monitor management] Make index template installation retry (elastic#125537)
  [Console] Support suggesting index templates v2 (elastic#124655)
  [Logs UI] Support switching between log source modes (elastic#124929)
  SavedObjects management: change index patterns labels to data views (elastic#125356)
  [ci-stats] add Client class for accessing test group stats (elastic#125164)
  [ML] Use Discover locator in Data Visualizer instead of URL Generator (elastic#124283)
  [Fleet] Add retries when starting docker registry in integration tests (elastic#125530)
  Update osquery.asciidoc to address doc issue (elastic#125425)
  synthetics e2e - use custom location when defined (elastic#125560)
  [ci] Retry failed steps in on-merge pipeline (elastic#125550)
  [babel] Bump babel packages (elastic#125446)
  [DOCS] Updates Upgrade Assistant API status (elastic#125410)
  [DOCS] Minor tweaks to upgrade docs (elastic#125538)
  [Uptime] handle null duration on ping list (elastic#125438)
  [TSVB][Lens] Navigate to Lens with your current configuration (elastic#114794)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/export_types/common/pdf/pdfmaker.ts
#	x-pack/plugins/reporting/server/export_types/printable_pdf/lib/generate_pdf.ts
#	x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts
kibanamachine added a commit that referenced this pull request Feb 15, 2022
* Add e2e test for service overview

* add test for comparison type change

* PR review comments addressed

* improve time range test

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 0e87ffc)

Co-authored-by: Miriam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants